Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces dynamic fixed-m constants and safe bounds calculation based on KV sequence length, adds support for virtual K-centering, and enhances input validation and dtype safety checks in the Pallas flash attention kernel. It also expands the test suite to cover various edge cases, including per-Q-block fallback, batched isolation, and virtual K-centering. The review feedback focuses on optimizing the TPU kernel performance by replacing expensive dynamic integer division with optimized BlockSpec mapping and introducing a static use_k_centering flag to conditionally compile the centering logic at trace time.
3b9d7aa to
cb1e4d1
Compare
There was a problem hiding this comment.
A few things before this can go in though:
- If this lands on its own, fixed-m gets turned off for R>1 (
use_fixed_m = Falsein_ulysses_ring_custom_attention, it only comes back in #478). That breaks the recipe we ship today. Can we split the stack so each PR is safe to merge by itself? - Cutting the Ulysses gate from 213 to 113 feels more conservative than we need. With centered keys, the mass you can lose is bounded by 2^(ceil(U)-C-126), so a gate around C+116 already keeps the loss under 0.1%. Could you share fallback rates so we can pick the number?
mk[0]means something different now, but a 2Dmkstill gets silently broadcast. Can we just raise on that?- The V check never fired in my runs on WAN 2.2 (every call passed), and it only exists because C moved from 88 to 102. Is it worth the extra complexity?
- Please put back the comments that explain the Mosaic cliff and k-smoothing. They save the next person a lot of pain.
syhuang22
left a comment
There was a problem hiding this comment.
Some line-level notes to go with my comment above.
44dfe63 to
fc91941
Compare
d57775a to
678e806
Compare
7bef907 to
0254d18
Compare
53905e1 to
4ec211c
Compare
4ec211c to
5345665
Compare
The merge-base changed after approval.
2b5c462 to
9b84d26
Compare
mbohlool
left a comment
There was a problem hiding this comment.
LGTM after addressing comments.
9b84d26 to
33ba2fc
Compare
…and safety fallbacks Implements exact fixed-m splash attention in Pallas on TPU. Instead of an online running max, a head / Q-block whose Cauchy-Schwarz bound max||q|| * max||k - k_bar|| is within a length-dependent window uses a precomputed m = ceil(bound) - C(N) (+ q . k_bar with virtual K-centering) and skips the per-block rescales; everything else keeps online softmax. - `get_fixed_m_constants(N)`: length-dependent recenter C(N) and safe bound that keep the f32 numerator/denominator from overflowing or flushing. - `fixed_m_dtype_is_safe`: rejects fp16/fp8 (exponent range too small for the recentered exponent); bf16 and fp32 pass. - V-magnitude gate: |V| <= DEFAULT_MAX_V_BOUND (256), otherwise online. - Per-(head, Q-block) dispatch in the kernel; the Ulysses and R=1 ring callers compile a uniform (all-fixed) and a hybrid kernel and pick one with `lax.cond` on `all_fixed`. Both kernels are compiled; the extra compile time / program size has not been measured. - On the R=1 `_ulysses_ring_custom_attention` path, when `batch > 1`, `devices_in_batch_sharding == 1`, and `(batch * num_kv_heads) % ulysses_shards == 0`, `batch` is folded into `heads` (`[1, B*H, S, D]` and `[1, B*Hkv, S, D]` before `shard_map`, with `k_mean` `[1, local_kv_heads, D]` computed inside `shard_map`) so `pallas_call` with scalar prefetch runs a single grid-batched kernel launch instead of a `vmap` serial loop. - Virtual K-centering: the kernel receives k_mean and adds q . k_bar to the fixed bound in VMEM (an MXU dot_general on an (Hkv, 8, d) tiled k_mean, precision=HIGHEST because the gate has no slack at its edge). K itself is never re-written in HBM. k_mean is padded to the lane-padded head_dim after `_pad_data_for_flash`, so head_dim < 128 works (regression test at d=64). `save_residuals=True` with `k_mean is not None` raises `NotImplementedError`; `make_splash_mha` checks `bool(interpret)` so `interpret=False` keeps the custom VJP. Explicit metadata contracts on the fixed-m ring path ---------------------------------------------------- 1. Norm representation is declared, not inferred. The gate previously guessed whether `fixed_m_norms` were squared with `(qn.max() * mk.max()) < 1000.0`. Magnitude cannot answer that: legacy unsquared norms of (1000, 2) have a true bound of 2000, but read as squared give sqrt(2000) ~= 44.7, a ~45x under-estimate that admits fixed-m where it must fall back. Replaced by a REQUIRED `fixed_m_norms_squared` argument (no default; omitting it with fixed-m on raises). Tests use squared norms. 2. The V-safety predicate is required, not assumed. The V-magnitude and dtype verdict cannot be re-derived from one hop's Q/K, so the ring kernel raises unless `v_ok` is passed (`value=None` in the metadata helper gives v_ok = 0, fail-closed). `_ulysses_ring_custom_attention` (R > 1) computes it on the PRE-all-to-all shards -- dtype safety plus |V| <= 256 -- and reduces it with pmin over BOTH internal axes (ring, ulysses): before the a2a each device holds a sequence shard of every head, and the ring's accumulate-vs-LSE branch must be taken uniformly by every ppermute participant. Only Q and K go through the optimization_barrier that keeps XLA from duplicating their producer chains into the norm reductions; V is reduced outside it so the Q/K a2a does not wait on V's producer. 3. Norm shape is validated against per_q_block. Both gates compute `qn * mk[:, None]`, so a (num_heads,) array supplied with per_q_block=True broadcasts to (num_heads, num_heads) and pairs head j's query norm with head h's key norm; a sink head then inherits a small bound and the kernel evaluates exp2(large_logit - small_m) -> inf. The kernel now rejects the mismatch. Tests - custom_splash_fixed_m_test.py (37): kernel unit tests with relative-L2 checks (including S < block_kv), k-norm (mk) shape and V-metadata fail-closed checks, `save_residuals + k_mean` and `interpret=False` VJP dispatch checks, `batch > 1` head-folding parity and HLO single-launch checks, integration through `_ulysses_attention` and the R=1 ring, and a U=2/R=2 check that a single V outlier on one pre-a2a device makes v_ok False on every device. - ring_fixed_m_test.py (19): 8 backend-independent contract tests (including the required fixed_m_norms_squared and v_ok arguments), 8 2-device ring kernel tests (5 numerical tests vs an f32 reference, 1 test that declared-unsquared and squared norms give the same result, and 2 adversarial overflow tests verifying `v_ok=False` forces finite output while `v_ok=True` overflows fp32 `pv`), and 3 tests showing the raw-K bound is unsound where the centered bound is not. - CI: all 56 tests across `custom_splash_fixed_m_test.py` (37) and `ring_fixed_m_test.py` (19) run unconditionally in GitHub Actions (using Pallas interpret mode on 4 CPU host devices when not on TPU). The new `end_to_end/tpu/run_wan_stack_tests.sh` runs all of them on a TPU VM; later PRs in this stack add their test files to it. Also pyink-reformats configuration_utils.py and wan_trainer_2_2.py, which fail the Linter workflow on main (formatting only; same hunk as the standalone chore PR so it rebases away once that merges).
05800ef to
a0e1afc
Compare
Summary
Fixed-m splash attention kernel. When a provable bound on every logit (
‖q‖·‖k‖, optionally centred on the mean key k̄) fits the fp32 exponent range, the kernel uses that fixed maxminstead of an online running max. This removes the per-block max/rescale work from the softmax inner loop. When the bound can't be proven, it falls back to the standard online-softmax kernel (_lse_scan).What's in it
custom_splash_attention.py: fixed-m kernel with dynamic Q/KV bounds. The in-kernelq·k̄dot runs atPrecision.HIGHEST, and_splash_attention_forward_ringrejects 2D(2, num_q_heads)mkmetadata in favor of 3D(2, num_q_heads, num_q_blocks).fixed_m_norms_squaredis a required argument.value=Nonegivesv_ok=False(fail-closed)._custom_ring_attention_forwardvalidatesfixed_m_norms[0]shape againstper_q_block.< flash_min_seq_length) falls back todot_product, cross-attention remaps totokamax_flash, andheads % num_shards != 0raisesValueError.Known limitations
pminover global-bound eligibility unless the caller passesuniform_fixed_m=True.Performance
Wan 2.2 T2V-A14B, 720p / 81 frames / 40 steps (
CP=4, DP=2, 1D UlyssesU=4,BQ=9472, BKV=4736, BKV_C=1024, DVFS unpinned) on TPU v6e-8:mainmain(f1d0fafb)ulysses_custom_fixed_mfeat/fixed-m-kernel)ulysses_custom_fixed_m_per_q_blockTests
custom_splash_fixed_m_test.py(31) andring_fixed_m_test.py(18):mkshape and V-metadata fail-closed checks;fixed_m_norms_squaredandv_okcontract tests (ring_fixed_m_test.py);N%8==0,N%bkv≠0, odd-multiple-of-8 tail), and a U=2/R=2 pre-all-to-all V-outlier check;GITHUB_ACTIONS=trueto keep CI fast;end_to_end/tpu/run_wan_stack_tests.shruns all 49 on a TPU VM.run_wan_stack_tests.sh, 170.4 s).Stack: #477 → #507 → #478 → #508 → #479 → #488 → #509 → #510 → #511 → #491